-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc: changed order of invocations in example. #9614
Conversation
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.
I'm not getting an error prior to this change. |
Not getting an error either. That said, I prefer this ordering. |
Yea, I do prefer this order too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with commit message fixed as I also don't get an error with current order.
|
||
req.on('error', (e) => { | ||
console.error(e); | ||
}); | ||
req.end(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this blank line.
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Landed in 550a958 Thanks for the contribution @atrioom |
@italoacasas the landed commit does not follow the commit message guidelines. |
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@lpinca I made some changes, I hope that one fit better. |
@italoacasas not a biggie but the second line ("When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.") is too long. Those lines should be wrapped at 72 chars. |
Hello and thank you for the help. I had this issue in a full grown application, I will try to reproduce it in a small test environment. Maybe add modules until I get the error. Thanks for accepting it anyways. |
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined. PR-URL: #9614 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Checklist
make -j8 test
(UNIX), orvcbuild test nosign
(Windows) passesAffected core subsystem(s)
doc
Description of change
When you call req.end() before you add .on listeners you get an Error that you can't call .on on undefined.